perf(stats): read local-path file sizes lazily per result by amondnet · Pull Request #92 · pleaseai/code-search

amondnet · 2026-09-04T10:18:08Z

Summary

Closes #90.

CspIndex::load_from_disk and from_path used to call compute_file_sizes eagerly, re-reading and UTF-16 counting every indexed file on every cached load, only to feed the file_chars side of token-savings telemetry. save_search_stats only ever looks up the unique file paths of the returned results.

This replaces the eager HashMap with a FileSizes value (crates/csp/src/indexing/file_sizes.rs):

Local-path indexes (from_path, load_from_disk with a still-present root) read sizes lazily per result and memoize them behind a Mutex, so the Arc<CspIndex> shared across MCP calls stays Sync.
Git sources (from_git) still capture sizes at clone time, because the temp checkout is gone by search time.
The path-traversal and regular-file guards from feat(stats): wire token-savings telemetry into search and find_related #82 move verbatim into read_file_chars, shared by both paths.
savings.jsonl record shape and UTF-16 accounting are unchanged.

This is a deliberate divergence from upstream semble, which computes sizes eagerly in SembleIndex.__init__. Recorded in .please/docs/references/semble.md §4.15.

Tests

file_sizes.rs: lazy read + memo (survives file deletion), None for .., absolute, symlink, and missing paths; captured map serves only known paths; empty() is not available.
index/tests.rs: from_path yields lazy sizes; load_from_disk with a removed source root reports no sizes.
Existing compute_file_sizes_* and save_search_stats tests unchanged in intent.

Verification

cargo fmt --all -- --check
cargo clippy --all-targets --all-features -- -D warnings
cargo test --workspace   # 286 lib + 22 CLI passed, 4 ignored (network)

Breaking change (library field, pre-1.0)

CspIndex::file_sizes is now a FileSizes value instead of a public HashMap<String, u64>, and stats::save_search_stats takes &FileSizes. Replace map lookups with index.file_sizes.get(path); there is no iteration or len() because local-path sizes are read on demand. The field is derived telemetry metadata and is not part of the README-documented API surface, so this ships without a major bump.
FileSizes is not Clone. Nothing in the workspace clones a CspIndex.

Follow-up

perf(stats): persist per-file sizes in the on-disk index instead of reading lazily #93 persists the size map in the on-disk index at build time. That removes the search-time read entirely, makes file_chars match the indexed content, and fixes git-from-cache and moved-source reporting file_chars: 0.

Review hardening

Lazy reads are bounded by MAX_FILE_BYTES, decode lossily like the indexer, memoize misses, and are contained to the canonicalized root with fstat on the opened handle.

Replace the eager per-index size map with FileSizes: a local source root is read on demand for the files a query actually returns, memoized behind a Mutex so the Arc<CspIndex> shared by MCP calls stays Sync. Git sources still capture sizes at clone time because the temp checkout is gone by search time. Cached loads no longer re-read every indexed file. Deliberate divergence from upstream semble (eager in SembleIndex.__init__); noted in the semble reference doc.

codacy-production · 2026-09-04T10:19:08Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 29 complexity · 4 duplication

Metric Results

Complexity 29

Duplication 4

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

codecov · 2026-09-04T10:20:53Z

Codecov Report

❌ Patch coverage is 98.95833% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/csp/src/indexing/file_sizes.rs	99.31%	1 Missing ⚠️
crates/csp/src/indexing/index.rs	93.33%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

gemini-code-assist

Code Review

This pull request introduces a lazy file size reader (FileSizes) to replace eager computation of all indexed file sizes, reading local source roots on demand per result with a memoized cache. Telemetry recording and CspIndex are updated to use this new struct, and comprehensive unit tests are added. The reviewer suggested optimizing compute_file_sizes by collecting unique file paths into a HashSet first to avoid redundant checks/reads across multiple chunks.

codspeed-hq · 2026-09-04T10:23:42Z

Merging this PR will not alter performance

✅ 5 untouched benchmarks

_{Comparing 90-perfstats-compute-file-sizes-lazily-for-local-path-indexes-instead-of-on-every-cached-load (437bf86) with main (8b7dd44)}

- bound lazy reads by MAX_FILE_BYTES and decode lossily like the indexer - memoize misses so an unreadable path is attempted once per index - dedupe result paths with a HashSet in save_search_stats - tighten telemetry tests to assert real file_chars values

greptile-apps · 2026-09-04T12:19:35Z

Greptile Summary

This PR replaces eager file-size collection with lazy, memoized reads for local indexes while retaining captured sizes for temporary Git checkouts.

Adds FileSizes with captured, lazy, and unavailable modes.
Resolves unique result paths while writing token-savings telemetry.
Keeps Git checkout sizes available after the temporary clone is removed.
Introduces a breaking public Rust API change and allows live filesystem state to diverge from indexed telemetry.
The lazy pathname read does not fully enforce its advertised containment and file-type guarantees.

Confidence Score: 4/5

The PR should not merge until the breaking public Rust API change is either preserved or explicitly handled; the lazy-read containment and telemetry-consistency issues should also be addressed.

Existing users of the published crate can fail to compile because two public contracts change types, while lazy source reads can escape the intended root and calculate savings from content different from the indexed results.

Files Needing Attention: crates/csp/src/indexing/index.rs, crates/csp/src/indexing/file_sizes.rs, crates/csp/src/stats.rs

Security Review

The lazy pathname read does not fully enforce source-root containment or regular-file and size checks. An indexed intermediate directory replaced with a symlink can redirect a telemetry read outside the configured source root, and the pathname can change between metadata validation and reading.

Important Files Changed

Filename	Overview
crates/csp/src/indexing/file_sizes.rs	Introduces lazy and captured size resolution, but live pathname reads can escape through intermediate symlinks and diverge from indexed content.
crates/csp/src/indexing/index.rs	Integrates lazy local sizing and captured Git sizing while changing the public `file_sizes` field contract.
crates/csp/src/stats.rs	Deduplicates result paths efficiently and resolves sizes through `FileSizes`, but changes a public function signature.
crates/csp/src/indexing/index/tests.rs	Adds focused coverage for lazy local sizing and unavailable sizes when a source root disappears.
crates/csp/src/bin/csp/main.rs	Strengthens the CLI telemetry test to verify nonzero serialized character counts.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    LP[Local source] --> LI[Build or load index]
    LI --> LZ[FileSizes lazy root]
    GS[Git source] --> CL[Temporary clone]
    CL --> CP[Capture file sizes]
    CP --> GI[Git-backed index]
    SR[Search results] --> UP[Unique result paths]
    UP --> GET[FileSizes get]
    LZ --> GET
    GI --> GET
    GET --> MEM[Memoized UTF-16 counts]
    MEM --> ST[Token-savings telemetry]

Prompt To Fix All With AI

### Issue 1
crates/csp/src/indexing/index.rs:89
**Public API Breakage**

Changing the public `CspIndex.file_sizes` field from `HashMap<String, u64>` to `FileSizes`, together with changing `save_search_stats` to accept `&FileSizes`, breaks existing consumers of the published Rust crate. Callers that iterate, clone, inspect, or pass the former map will now fail to compile. Preserve the existing contract or handle and document this as a deliberate breaking API change.

### Issue 2
crates/csp/src/indexing/file_sizes.rs:104-110
**Path Checks Are Bypassable**

The containment and file-type checks do not cover the later pathname read. `symlink_metadata` follows symlinks in intermediate directories, and the path can also change before `std::fs::read` reopens it. A mutable indexed tree can therefore redirect telemetry reads outside the source root or bypass the regular-file and size checks. Use a no-follow, descriptor-relative open and enforce the byte limit while reading.

**How this was verified:** An indexed intermediate directory can be replaced with a symlink, after which metadata and the read both resolve a regular target outside the configured source root.

### Issue 3
crates/csp/src/indexing/file_sizes.rs:63-68
**Telemetry Uses Mismatched Content**

Lazily reading the live source can make `file_chars` describe different content from the returned indexed chunks. During the MCP freshness cooldown, or when an explicitly loaded index remains open while its source changes, the first lookup reads the modified file. Files grown beyond `MAX_FILE_BYTES` also contribute zero. As a result, `saved_chars = file_chars - snippet_chars` becomes inaccurate. Preserve the size associated with indexed content or invalidate lazy values whenever the index becomes stale.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "chore(stats): apply code review fixes to..." | Re-trigger Greptile}

Collect unique chunk paths first so an unreadable file is attempted once instead of once per chunk (Gemini review on #92).

amondnet · 2026-09-04T12:20:00Z

/gemini review

Reject a symlink leaf, require the canonicalized path to stay under the canonicalized root (a symlinked intermediate directory no longer escapes), fstat the opened handle instead of the path, and cap the read at MAX_FILE_BYTES while reading (Greptile review on #92).

gemini-code-assist

Code Review

This pull request implements lazy loading and memoization of file sizes for token-savings telemetry via a new FileSizes utility, replacing the previous eager computation. It includes path traversal guards, file size limits, and robust test coverage. The review feedback suggests a performance optimization in read_file_chars to count UTF-16 code units using char::len_utf16 instead of performing a full encoding.

amondnet · 2026-09-04T12:22:37Z

/gemini review

…d_file_chars

cubic-dev-ai

Review completed against the latest diff

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

gemini-code-assist

Code Review

This pull request introduces lazy loading and memoization of file character counts via a new FileSizes utility to optimize token-savings telemetry, replacing the previous eager computation. The feedback focuses on performance improvements to avoid redundant filesystem I/O: specifically, the root path should be canonicalized once during initialization in FileSizes::lazy and at the start of compute_file_sizes, rather than repeatedly canonicalizing it inside read_file_chars for every file lookup.

FileSizes::lazy and compute_file_sizes canonicalize the root at construction; read_file_chars now requires a canonical root and does a plain prefix check (Gemini review on #92).

amondnet · 2026-09-04T12:26:39Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces lazy loading of file sizes (character counts) for token-savings telemetry, diverging from the upstream eager calculation. It adds a new FileSizes struct in crates/csp/src/indexing/file_sizes.rs that supports both eager (captured) and lazy (on-demand from a local root) resolution with memoization, path traversal guards, and file size limits. Corresponding updates and tests have been added across the indexing, stats, and main binary modules. I have no feedback to provide as there are no review comments to assess.

cubic-dev-ai

All reported issues were addressed across 2 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

… reads A FIFO at an indexed path made File::open block until a writer appeared, stalling the search that triggered the size lookup. Check the canonical path with symlink_metadata first; the fstat on the opened handle stays as the race guard. Raised by cubic on #92.

sonarqubecloud · 2026-09-04T12:35:52Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
99.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

amondnet linked an issue Sep 4, 2026 that may be closed by this pull request

perf(stats): compute file sizes lazily for local-path indexes instead of on every cached load #90

Closed

gemini-code-assist Bot reviewed Sep 4, 2026

View reviewed changes

Comment thread crates/csp/src/indexing/index.rs Outdated

amondnet marked this pull request as ready for review September 4, 2026 12:16

chore(stats): dedup paths before reading sizes in compute_file_sizes

17825ab

Collect unique chunk paths first so an unreadable file is attempted once instead of once per chunk (Gemini review on #92).

greptile-apps Bot reviewed Sep 4, 2026

View reviewed changes

Comment thread crates/csp/src/indexing/index.rs

Comment thread crates/csp/src/indexing/file_sizes.rs Outdated

Comment thread crates/csp/src/indexing/file_sizes.rs

amondnet mentioned this pull request Sep 4, 2026

perf(stats): persist per-file sizes in the on-disk index instead of reading lazily #93

Open

gemini-code-assist Bot reviewed Sep 4, 2026

View reviewed changes

Comment thread crates/csp/src/indexing/file_sizes.rs

docs(stats): document the residual canonicalize-then-open race in rea…

48dab11

…d_file_chars

cubic-dev-ai Bot reviewed Sep 4, 2026

View reviewed changes

Comment thread crates/csp/src/indexing/index.rs

Comment thread crates/csp/src/indexing/file_sizes.rs

gemini-code-assist Bot reviewed Sep 4, 2026

View reviewed changes

Comment thread crates/csp/src/indexing/file_sizes.rs

Comment thread crates/csp/src/indexing/file_sizes.rs

Comment thread crates/csp/src/indexing/index.rs

perf(stats): canonicalize the size root once instead of per read

646cfc8

FileSizes::lazy and compute_file_sizes canonicalize the root at construction; read_file_chars now requires a canonical root and does a plain prefix check (Gemini review on #92).

gemini-code-assist Bot reviewed Sep 4, 2026

View reviewed changes

cubic-dev-ai Bot reviewed Sep 4, 2026

View reviewed changes

Comment thread crates/csp/src/indexing/file_sizes.rs

amondnet merged commit 1acd823 into main Sep 4, 2026
13 checks passed

amondnet deleted the 90-perfstats-compute-file-sizes-lazily-for-local-path-indexes-instead-of-on-every-cached-load branch September 4, 2026 12:39

pleaseai-release-bot Bot mentioned this pull request Sep 4, 2026

chore(main): release 0.1.10 #72

Merged

amondnet added a commit that referenced this pull request Sep 4, 2026

Merge origin/main (#92 lazy file sizes) into incremental reindexing

4c5c1cf

Uh oh!

Conversation

amondnet commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Verification

Breaking change (library field, pre-1.0)

Follow-up

Review hardening

Uh oh!

codacy-production Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

codecov Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

codspeed-hq Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

greptile-apps Bot commented Sep 4, 2026

Greptile Summary

Confidence Score: 4/5

Security Review

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amondnet commented Sep 4, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

amondnet commented Sep 4, 2026

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amondnet commented Sep 4, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Sep 4, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

amondnet commented Sep 4, 2026 •

edited

Loading

codacy-production Bot commented Sep 4, 2026 •

edited

Loading

codecov Bot commented Sep 4, 2026 •

edited

Loading

codspeed-hq Bot commented Sep 4, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading